Backend typescript improvements#1803
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR systematically hardens TypeScript type safety across the backend by adding stricter compiler options, creating a safe error-message extraction helper, standardizing error handling patterns with type casts throughout 50+ files, and introducing record-based object casting for dynamic property access. ChangesTypeScript Strictness & Error Handling Standardization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens backend TypeScript checking and updates affected source/test code so the backend can be typechecked in CI.
Changes:
- Adds stricter backend TypeScript configs and a
typecheckscript wired into code-quality CI. - Updates many catch blocks and dynamic property accesses to satisfy stricter TypeScript options.
- Adds a
basic-authambient declaration and fixes a demo-data action-rule property access.
Reviewed changes
Copilot reviewed 62 out of 63 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/code-quality.yml |
Runs backend typecheck in CI. |
backend/package.json |
Adds typecheck and lint scripts. |
backend/tsconfig.json |
Enables additional TypeScript checks and removes allowJs. |
backend/tsconfig.src.json |
Adds stricter source-only typecheck config. |
backend/src/helpers/get-error-message.ts |
Adds unknown-safe error message helpers. |
backend/src/types/basic-auth.d.ts |
Adds local typings for basic-auth. |
backend/src/entities/demo-data/demo-data.service.ts |
Updates error handling and fixes action-rule title lookup. |
backend/src/entities/ai/ai.service.ts |
Uses error-message helper in AI processing paths. |
backend/src/entities/cron-jobs/cron-jobs.service.ts |
Uses safer error message/stack handling. |
backend/src/entities/table/use-cases/add-row-in-table.use.case.ts |
Updates caught error message access. |
backend/src/entities/table/use-cases/* |
Adjusts table use-case error handling and dynamic indexing. |
backend/src/entities/connection/use-cases/* |
Adjusts connection use-case indexing/error handling. |
backend/src/entities/table-actions/** |
Adds explicit callback/error-related types. |
backend/src/entities/table-settings/** |
Adds DTO typing and dynamic cleanup casts. |
backend/src/entities/table-schema/** |
Updates cleanup callbacks and null typing. |
backend/src/entities/table-logs/** |
Types query params and dynamic log-field masking. |
backend/src/entities/cedar-authorization/** |
Adds explicit array/error typings. |
backend/src/entities/visualizations/** |
Updates AI visualization error handling. |
backend/src/entities/widget/table-widget.entity.ts |
Updates widget option catch logging. |
backend/src/entities/email/email/email.service.ts |
Updates reminder catch logging. |
backend/src/entities/shared-jobs/shared-jobs.service.ts |
Updates shared-job catch logging. |
backend/src/shared/** |
Updates config/turnstile catch handling. |
backend/test/** |
Updates tests/utilities for catch variable typing and regex precision. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+197
to
+200
| message: (e as Error).message.includes('duplicate key value') | ||
| ? Messages.CANT_INSERT_DUPLICATE_KEY | ||
| : `${Messages.FAILED_ADD_ROW_IN_TABLE} | ||
| ${Messages.ERROR_MESSAGE} ${e.message} ${Messages.TRY_AGAIN_LATER}`, | ||
| ${Messages.ERROR_MESSAGE} ${(e as Error).message} ${Messages.TRY_AGAIN_LATER}`, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/entities/table-logs/table-logs.service.ts (1)
51-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInconsistent type-casting pattern with
createAndSaveNewLogsUtilmethod.This method uses
Record<string, unknown>casts for sensitive field redaction, but the similar methodcreateAndSaveNewLogsUtil(lines 146-180) still uses direct property access without casts. This inconsistency will likely cause TypeScript errors with strict compiler settings.🔧 Recommended fix
Apply the same casting pattern to
createAndSaveNewLogsUtil. In the sensitive field loop starting at line 148, add:const sensitive_fields = tableSettings?.sensitive_fields || []; const tableLogsEntities: Array<TableLogsEntity> = []; for (const logData of logsData) { const { row, old_data } = logData; + const oldRec = old_data as Record<string, unknown>; + const rowRec = row as Record<string, unknown>; for (const fieldName of sensitive_fields) { if ( old_data && typeof old_data === 'object' && isObjectPropertyExists(old_data, fieldName) && row && typeof row === 'object' && isObjectPropertyExists(row, fieldName) ) { // eslint-disable-next-line security/detect-object-injection - if (this.compareValues(old_data[fieldName], row[fieldName])) { + if (this.compareValues(oldRec[fieldName], rowRec[fieldName])) { // eslint-disable-next-line security/detect-object-injection - old_data[fieldName] = Constants.REMOVED_SENSITIVE_FIELD_IF_CHANGED; + oldRec[fieldName] = Constants.REMOVED_SENSITIVE_FIELD_IF_CHANGED; // eslint-disable-next-line security/detect-object-injection - row[fieldName] = Constants.REMOVED_SENSITIVE_FIELD_IF_CHANGED; + rowRec[fieldName] = Constants.REMOVED_SENSITIVE_FIELD_IF_CHANGED; } else { // Apply similar changes to remaining branches...Continue the pattern for all remaining
old_data[fieldName]androw[fieldName]accesses in that method.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/table-logs/table-logs.service.ts` around lines 51 - 84, The createAndSaveNewLogsUtil method has inconsistent property access for sensitive field redaction; update its sensitive field loop to mirror the pattern used earlier by casting old_data and row to Record<string, unknown> (e.g., const oldRec = old_data as Record<string, unknown>; const rowRec = row as Record<string, unknown>), then replace direct old_data[fieldName] and row[fieldName] accesses with oldRec[fieldName] and rowRec[fieldName], keep the existing isObjectPropertyExists/type checks and the same Constants.REMOVED_SENSITIVE_FIELD_* assignments, and ensure all occurrences in createAndSaveNewLogsUtil use the cast variables to satisfy strict TypeScript checks.
🧹 Nitpick comments (12)
backend/src/entities/connection/use-cases/update-connection.use.case.ts (1)
65-76: 💤 Low valueMinor optimization: cast accumulator once outside the reduce.
Currently,
accRecis cast on every iteration. While this doesn't affect correctness, casting once before the reduce would be slightly more efficient.♻️ Optional optimization
const keysToKeep = ['title', 'schema', ...booleanKeys]; const params = connection_parameters as unknown as Record<string, unknown>; -connection_parameters = Object.keys(connection_parameters).reduce( +const result: Record<string, unknown> = {}; +connection_parameters = Object.keys(connection_parameters).reduce( (acc, key) => { - const accRec = acc as unknown as Record<string, unknown>; // eslint-disable-next-line security/detect-object-injection if (params[key] || keysToKeep.includes(key)) { // eslint-disable-next-line security/detect-object-injection - accRec[key] = params[key]; + (result as Record<string, unknown>)[key] = params[key]; } - return acc; + return result as UpdateConnectionDs['connection_parameters']; }, {} as UpdateConnectionDs['connection_parameters'], );Alternatively, keep the current approach if readability is preferred.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/connection/use-cases/update-connection.use.case.ts` around lines 65 - 76, The reduce callback is casting the accumulator on every iteration; to optimize, create a correctly typed variable for the initial accumulator before calling reduce (e.g., const initialAcc = {} as UpdateConnectionDs['connection_parameters']) and pass that into connection_parameters = Object.keys(connection_parameters).reduce(..., initialAcc), then remove the per-iteration cast and use the accumulator directly; keep existing checks involving params and keysToKeep and the same property assignment logic inside the reducer.backend/src/entities/table-filters/use-cases/update-table-filters.use.case.ts (1)
55-63: 💤 Low valueConsider extracting the undefined-key deletion pattern.
This pattern is duplicated in
update-table-settings.use.case.ts(lines 56-63). A shared helper could reduce duplication, though the pattern is simple enough that duplication may be acceptable.♻️ Optional helper function
// backend/src/helpers/remove-undefined-properties.ts export function removeUndefinedProperties<T extends object>(obj: T): T { const objRec = obj as unknown as Record<string, unknown>; for (const key in obj) { // eslint-disable-next-line security/detect-object-injection if (objRec[key] === undefined) { // eslint-disable-next-line security/detect-object-injection delete objRec[key]; } } return obj; }Then use:
removeUndefinedProperties(updatedFilterEntity);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/table-filters/use-cases/update-table-filters.use.case.ts` around lines 55 - 63, The duplicated loop that removes undefined properties from entities (seen around updatedFilterEntity built by buildNewTableFiltersEntity in update-table-filters.use.case.ts and the same pattern in update-table-settings.use.case.ts) should be extracted to a small reusable helper (e.g., removeUndefinedProperties<T extends object>(obj: T): T) exported from a new helpers module; replace the inline loop in both use cases with a call to removeUndefinedProperties(updatedFilterEntity) (and the equivalent variable in the settings use case), keeping the same type signature and any necessary eslint disable comments inside the helper so behavior and linting remain identical.backend/src/entities/connection/use-cases/find-all-connections.use.case.ts (1)
109-117: ⚡ Quick winConsider extracting the duplicate
filterConnectionKeyshelper.This exact helper function is duplicated in
find-one-connection.use.case.ts(lines 66-74). Extracting it to a shared utility would improve maintainability and reduce duplication.♻️ Suggested refactor
Create a new utility file, e.g.,
backend/src/entities/connection/utils/filter-connection-keys.ts:import { ConnectionEntity } from '../connection.entity.js'; import { FilteredConnection } from '../use-cases/find-all-connections.use.case.js'; export function filterConnectionKeys( connection: ConnectionEntity, allowedKeys: Array<string> ): FilteredConnection { return Object.keys(connection).reduce((acc, key) => { if (allowedKeys.includes(key)) { // eslint-disable-next-line security/detect-object-injection (acc as Record<string, unknown>)[key] = (connection as unknown as Record<string, unknown>)[key]; } return acc; }, {} as FilteredConnection); }Then import and use it in both files instead of defining it locally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/connection/use-cases/find-all-connections.use.case.ts` around lines 109 - 117, The helper filterConnectionKeys is duplicated between find-all-connections.use.case and find-one-connection.use.case; extract it into a shared utility (e.g., filterConnectionKeys) that accepts (connection: ConnectionEntity, allowedKeys: string[]) and returns FilteredConnection, export it, then replace the local copies in both use-case files with an import of the shared function and remove the duplicated definitions; ensure the utility uses the same type imports (ConnectionEntity and FilteredConnection) and preserves the existing behavior including the eslint disable comment.backend/src/entities/ai/use-cases/request-info-from-table-with-ai-v7.use.case.ts (3)
118-118: ⚡ Quick winConsider using the
getErrorMessagehelper for consistency.The codebase now provides a
getErrorMessagehelper (inbackend/src/helpers/get-error-message.ts) for safely extracting error messages fromunknowncatch variables. Using it here would align with the pattern used inai.service.tsand improve maintainability.Additionally, the optional chaining
?.is redundant after casting toError, sinceErrorinstances always have amessageproperty.♻️ Refactor to use the error helper
+import { getErrorMessage } from '../../../helpers/get-error-message.js'; ... - await slackPostMessage((error as Error)?.message); + await slackPostMessage(getErrorMessage(error));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/ai/use-cases/request-info-from-table-with-ai-v7.use.case.ts` at line 118, Replace the direct cast and optional chaining when posting the caught error to Slack with the shared helper: use getErrorMessage(error) and pass its return to slackPostMessage instead of (error as Error)?.message; update the call in request-info-from-table-with-ai-v7.use.case.ts (where slackPostMessage is invoked) to await slackPostMessage(getErrorMessage(error)) so it safely handles unknown catch values and follows the existing ai.service.ts pattern.
293-295: ⚡ Quick winConsider using the
getErrorMessagehelper for consistency.For consistency with the new error-handling pattern, consider using
getErrorMessage(error)instead of casting manually.♻️ Refactor to use the error helper
- const errMessage = (error as Error).message; - this.logger.error(`Tool call ${toolCall.name} (${toolCall.id}) failed: ${errMessage}`); - result = encodeError({ error: errMessage }); + const errMessage = getErrorMessage(error); + this.logger.error(`Tool call ${toolCall.name} (${toolCall.id}) failed: ${errMessage}`); + result = encodeError({ error: errMessage });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/ai/use-cases/request-info-from-table-with-ai-v7.use.case.ts` around lines 293 - 295, Replace the manual cast and message extraction with the shared helper: call getErrorMessage(error) to obtain errMessage, use that value in this.logger.error(`Tool call ${toolCall.name} (${toolCall.id}) failed: ${errMessage}`) and in result = encodeError({ error: errMessage }); ensure getErrorMessage is imported into the module (or available in the current scope) and remove the (error as Error).message cast.
195-195: ⚡ Quick winConsider using the
getErrorMessagehelper for consistency.For consistency with the new error-handling pattern, consider using
getErrorMessage(loopError)instead of casting manually.♻️ Refactor to use the error helper
- this.logger.error(`Error in tool loop at depth ${depth + 1}: ${(loopError as Error).message}`); + this.logger.error(`Error in tool loop at depth ${depth + 1}: ${getErrorMessage(loopError)}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/ai/use-cases/request-info-from-table-with-ai-v7.use.case.ts` at line 195, Replace the manual cast and .message access with the shared error helper for consistency: inside the tool loop error handling where this.logger.error is called (the statement currently using `(loopError as Error).message`), call getErrorMessage(loopError) and include its return value in the logged message so the log uses the standardized error formatting.backend/src/entities/cron-jobs/cron-jobs.service.ts (2)
105-110: ⚡ Quick winConsider using the error helper functions for consistency.
Similar to the inner error handling above, this block manually normalizes the error. Using
getErrorMessageandgetErrorStackwould maintain consistency with the rest of the codebase.♻️ Refactor to use error helpers
- const err = e instanceof Error ? e : new Error(String(e)); - const errorMessage = err.stack ? `${err.message}\n${err.stack.split('\n').slice(0, 5).join('\n')}` : err.message; + const message = getErrorMessage(e); + const stack = getErrorStack(e); + const errorMessage = stack ? `${message}\n${stack.split('\n').slice(0, 5).join('\n')}` : message;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/cron-jobs/cron-jobs.service.ts` around lines 105 - 110, Replace the manual error normalization in the main cron handler catch block with the shared helpers: call getErrorMessage(e) and getErrorStack(e) to produce a consistent message and stack, then pass a combined string to slackPostMessage(`Error in morning cron: ${message}\n${stack}`, Constants.EXCEPTIONS_CHANNELS); still call Sentry.captureException(e) and keep the console.error('Main cron handler error:', e) line; update the catch in cron-jobs.service.ts to use getErrorMessage and getErrorStack instead of the inline instanceof/Error conversion and stack slicing.
96-104: ⚡ Quick winConsider using the error helper functions for consistency.
This block manually normalizes
innerErrorto anErrorinstance and extracts the stack, but the codebase now hasgetErrorMessageandgetErrorStackhelpers (imported at line 9) that encapsulate this logic. Using them would simplify the code and maintain consistency with line 83.♻️ Refactor to use error helpers
- const err = innerError instanceof Error ? innerError : new Error(String(innerError)); - const errorMessage = err.stack - ? `${err.message}\n${err.stack.split('\n').slice(0, 5).join('\n')}` - : err.message; + const message = getErrorMessage(innerError); + const stack = getErrorStack(innerError); + const errorMessage = stack + ? `${message}\n${stack.split('\n').slice(0, 5).join('\n')}` + : message;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/cron-jobs/cron-jobs.service.ts` around lines 96 - 104, Replace the manual error normalization and stack-slicing with the existing helpers: call getErrorMessage(innerError) and getErrorStack(innerError) to build the Slack message (e.g., `${getErrorMessage(innerError)}\n${getErrorStack(innerError)}` or slice the stack lines via getErrorStack if needed), then pass the original innerError to Sentry.captureException(innerError) and post to slackPostMessage(..., Constants.EXCEPTIONS_CHANNELS); remove the ad-hoc new Error(...) creation and manual stack handling to keep consistency with the error handling used at line 83.backend/src/entities/table/use-cases/find-tables-in-connection-v2.use.case.ts (1)
47-57: ⚡ Quick winError message extraction could be more robust.
The error message is extracted once into
errMessage(good pattern to avoid repeated casts), but the cast still assumes the error is an Error instance. If a non-Error is thrown,errMessagewill beundefinedand neither equality check will match, causing the error to propagate uncaught after the catch block.Consider using
getErrorMessage(error)to safely handle any thrown value type.♻️ Safer error message extraction
+import { getErrorMessage } from '../../../helpers/get-error-message.js'; try { connection = await this._dbContext.connectionRepository.findAndDecryptConnection(connectionId, masterPwd); } catch (error) { - const errMessage = (error as Error).message; + const errMessage = getErrorMessage(error); if (errMessage === Messages.MASTER_PASSWORD_MISSING) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/table/use-cases/find-tables-in-connection-v2.use.case.ts` around lines 47 - 57, The catch block in the FindTablesInConnectionV2 use case extracts the error message via (error as Error).message which breaks if a non-Error is thrown; replace that extraction with a safe helper (getErrorMessage(error)) and use the returned string for the comparisons against Messages.MASTER_PASSWORD_MISSING and Messages.MASTER_PASSWORD_INCORRECT so all thrown values are handled robustly and the existing HttpException branches (type 'no_master_key', HttpStatus.BAD_REQUEST, etc.) still run when appropriate.backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts (1)
122-130: ⚡ Quick winConsider using
getErrorMessagehelper for consistent error reporting.Both the Slack debug message and the
originalMessagefield use(error as Error).message, which could returnundefinedfor non-Error thrown values, resulting in degraded error reporting in both monitoring and user-facing responses.♻️ More robust error message extraction
+import { getErrorMessage } from '../../../helpers/get-error-message.js'; } catch (error) { operationResult = OperationResultStatusEnum.unsuccessfully; if (error instanceof HttpException) { throw error; } + const errorMessage = getErrorMessage(error); // todo: temporary debug log await slackPostMessage(` - CSV Export Failed with error: ${(error as Error).message}\n + CSV Export Failed with error: ${errorMessage}\n Connection type: ${connection.type}\n SSH Option: ${connection.ssh}\n SSL Option: ${connection.ssl}\n `); throw new HttpException( { message: Messages.CSV_EXPORT_FAILED, - originalMessage: (error as Error).message, + originalMessage: errorMessage, }, HttpStatus.INTERNAL_SERVER_ERROR, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts` around lines 122 - 130, Replace direct casts of (error as Error).message with the centralized getErrorMessage(error) helper to ensure consistent, non-undefined error text; update the Slack/debug message string interpolation and the HttpException payload's originalMessage where Messages.CSV_EXPORT_FAILED is thrown (inside the exportCsvFromTable use-case), and add/import getErrorMessage at the top of the file if not already present so both logging and the HttpException use the helper-derived message.backend/src/entities/table-settings/common-table-settings/repository/table-settings-custom-repository-extension.ts (1)
57-57: ⚡ Quick winConsider using the
getErrorMessagehelper for safer error message extraction.The type cast
(e as Error)assumes the caught value is an Error instance, but JavaScript allows throwing any value. If a non-Error is thrown,.messagewill be undefined, resulting in poor log output.The PR introduces a
getErrorMessagehelper inbackend/src/helpers/get-error-message.tsthat safely extracts messages from unknown error types. Using it here would improve robustness and consistency with other error handling in the codebase.♻️ Safer error message extraction
+import { getErrorMessage } from '../../../helpers/get-error-message.js'; try { return await qb.getOne(); } catch (e) { - console.info(`Table setting not found exception. => `, (e as Error).message); + console.info(`Table setting not found exception. => `, getErrorMessage(e)); throw new HttpException(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/table-settings/common-table-settings/repository/table-settings-custom-repository-extension.ts` at line 57, Replace the direct cast (e as Error).message usage with the shared getErrorMessage helper to safely extract the error text; import getErrorMessage from backend/src/helpers/get-error-message.ts into table-settings-custom-repository-extension.ts and update the catch/logging line that currently calls console.info(`Table setting not found exception. => `, (e as Error).message) to use getErrorMessage(e) so the log always prints a useful message even when non-Error values are thrown.backend/src/shared/services/turnstile.service.ts (1)
46-47: ⚡ Quick winConsider using
getErrorMessagehelper, or document the axios-specific error handling.The custom type shape handles axios error structure (
response.data). While this is more sophisticated than a simple cast, using thegetErrorMessagehelper would provide consistency. However, ifresponse.datacontains critical error details not available in.message, the current approach may be justified.💡 Option 1: Use getErrorMessage (simpler, consistent)
+import { getErrorMessage } from '../../helpers/get-error-message.js'; + } catch (error) { if (error instanceof BadRequestException) throw error; - const err = error as { response?: { data?: unknown }; message?: string }; - console.error('Turnstile verification error:', err?.response?.data || err?.message || error); + console.error('Turnstile verification error:', getErrorMessage(error)); throw new BadRequestException('Turnstile verification failed. Please try again.'); }Option 2: If
response.datais genuinely important for debugging, document why this special case exists with a comment explaining the axios error structure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/shared/services/turnstile.service.ts` around lines 46 - 47, Replace the ad-hoc axios-error casting and console.error call with the standardized getErrorMessage helper (use getErrorMessage(error)) inside the Turnstile verification error handling so logs are consistent with other services; if response.data truly contains extra diagnostics you need, keep the existing err cast but add a short comment above it explaining why axios-specific response.data is required for debugging and why we deviate from getErrorMessage (reference the local err variable and the Turnstile verification error log statement).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/entities/demo-data/demo-data.service.ts`:
- Around line 44-47: The catch block in createDemoDataForUser currently logs and
posts to Slack but swallows the error, causing the function to resolve with
undefined instead of propagating failures; update the catch in
createDemoDataForUser to re-throw the caught error (after console.error and
await slackPostMessage) so callers receive the exception and the function's
contract (Array<ConnectionEntity>) isn’t silently broken.
In `@backend/src/entities/table-logs/table-logs.controller.ts`:
- Line 71: The `@Query`() parameter typing in table-logs.controller.ts (the
parameter named query) wrongly assumes all query values are strings; change its
type from Record<string, string> to Record<string, string | string[]> (or
replace with a DTO that models array-valued params) so repeated query keys like
?a=1&a=2 are handled; update any other `@Query`() parameters in this controller
that use Record<string, string> to the same Record<string, string | string[]>
(or corresponding DTO) to keep typings consistent.
In `@backend/src/entities/table/use-cases/add-row-in-table.use.case.ts`:
- Around line 197-200: The error handler in add-row-in-table.use.case.ts
currently directly accesses (e as Error).message and calls .includes which can
throw if a non-Error is thrown; replace that direct access with the shared
getErrorMessage helper to safely extract the message, then perform the
duplicate-key check against the returned string (e.g., const msg =
getErrorMessage(e); if (msg.includes('duplicate key value')) ...) and use msg
when composing the fallback Messages.FAILED_ADD_ROW_IN_TABLE error text so the
handler cannot crash when a non-Error is thrown.
In
`@backend/src/entities/visualizations/panel-position/use-cases/generate-panel-position-with-ai.use.case.ts`:
- Line 288: Replace unsafe casts of caught errors to Error with the shared
helper: import getErrorMessage from 'backend/src/helpers/get-error-message' at
the top of generate-panel-position-with-ai.use.case.ts, then change any
occurrences that set result = encodeError({ error: (error as Error).message })
(and the similar lines around the catch blocks near the current diff positions)
to use encodeError({ error: getErrorMessage(error) }) or simply
encodeError(getErrorMessage(error)) consistent with existing encoding patterns;
update all three catch sites (the ones flagged at the current diff and the ones
around lines 309 and 385) so they call getErrorMessage(error) instead of casting
to Error.
In
`@backend/src/entities/visualizations/panel-position/use-cases/generate-table-dashboard-with-ai.use.case.ts`:
- Line 164: Replace unsafe casts of caught exceptions like (error as
Error).message with the safe getErrorMessage helper: import getErrorMessage from
its module at the top of generate-table-dashboard-with-ai.use.case.ts, then
update each catch block that currently does this.logger.warn(`Panel
"${panel.name}" skipped: ${(error as Error).message}`) (and the three other
similar logger.warn calls in the same file) to call getErrorMessage(error) and
include its return value in the log message instead; ensure all four occurrences
use getErrorMessage for consistent, safe error extraction.
In `@backend/src/entities/visualizations/panel/panel.entity.ts`:
- Line 58: Replace unsafe casts to Error in the entity lifecycle hooks by
importing and using the getErrorMessage helper; add an import for
getErrorMessage at the top of panel.entity.ts and in the three lifecycle hook
methods (the ones that currently call console.error('-> Error widget options
stringify ' + (e as Error).message) and the two similar lines) change those
message constructions to use getErrorMessage(e) so the log calls become
console.error('-> Error widget options stringify ' + getErrorMessage(e)) (apply
the same replacement for the other two hooks).
In `@backend/src/entities/widget/table-widget.entity.ts`:
- Line 54: Replace the unsafe manual error casts in the TableWidget entity
lifecycle hooks with the shared getErrorMessage helper: add an import for
getErrorMessage and update the three places where errors are logged (the
console.error call at the location shown and the two other lifecycle-hook error
logs referenced around the same file) to call getErrorMessage(e) instead of
casting (e as Error). Ensure each log message uses the helper to extract the
message and keep the existing descriptive text.
In `@backend/src/helpers/validators/is-action-url-host-allowed.ts`:
- Line 40: Replace the direct cast in the catch block with the safe helper: add
an import for getErrorMessage at the top of the module and change the
console.error call inside the catch in is-action-url-host-allowed (the catch
that currently logs (e as Error).message) to use getErrorMessage(e) instead so
unknown thrown values are handled safely.
In `@backend/src/shared/config/app-config.ts`:
- Line 279: Replace the unsafe cast (writeError as Error).message with the safer
helper getErrorMessage: import getErrorMessage at the top, then call
getErrorMessage(writeError) inside the console.warn so the warning uses
getErrorMessage(writeError) instead of (writeError as Error).message; this keeps
the existing message text and context but extracts the error string robustly for
the writeError variable in the same block.
In `@backend/test/ava-tests/saas-tests/custom-domains-e2e.test.ts`:
- Line 131: Replace direct casts to Error in the test catch blocks with the
shared helper: import getErrorMessage at the top of
backend/test/ava-tests/saas-tests/custom-domains-e2e.test.ts and change each
t.fail((error as Error).message) to t.fail(getErrorMessage(error)); update all
eight catch sites (current occurrences at the indicated lines) so tests use
getErrorMessage(error) for safe, consistent error extraction.
---
Outside diff comments:
In `@backend/src/entities/table-logs/table-logs.service.ts`:
- Around line 51-84: The createAndSaveNewLogsUtil method has inconsistent
property access for sensitive field redaction; update its sensitive field loop
to mirror the pattern used earlier by casting old_data and row to Record<string,
unknown> (e.g., const oldRec = old_data as Record<string, unknown>; const rowRec
= row as Record<string, unknown>), then replace direct old_data[fieldName] and
row[fieldName] accesses with oldRec[fieldName] and rowRec[fieldName], keep the
existing isObjectPropertyExists/type checks and the same
Constants.REMOVED_SENSITIVE_FIELD_* assignments, and ensure all occurrences in
createAndSaveNewLogsUtil use the cast variables to satisfy strict TypeScript
checks.
---
Nitpick comments:
In
`@backend/src/entities/ai/use-cases/request-info-from-table-with-ai-v7.use.case.ts`:
- Line 118: Replace the direct cast and optional chaining when posting the
caught error to Slack with the shared helper: use getErrorMessage(error) and
pass its return to slackPostMessage instead of (error as Error)?.message; update
the call in request-info-from-table-with-ai-v7.use.case.ts (where
slackPostMessage is invoked) to await slackPostMessage(getErrorMessage(error))
so it safely handles unknown catch values and follows the existing ai.service.ts
pattern.
- Around line 293-295: Replace the manual cast and message extraction with the
shared helper: call getErrorMessage(error) to obtain errMessage, use that value
in this.logger.error(`Tool call ${toolCall.name} (${toolCall.id}) failed:
${errMessage}`) and in result = encodeError({ error: errMessage }); ensure
getErrorMessage is imported into the module (or available in the current scope)
and remove the (error as Error).message cast.
- Line 195: Replace the manual cast and .message access with the shared error
helper for consistency: inside the tool loop error handling where
this.logger.error is called (the statement currently using `(loopError as
Error).message`), call getErrorMessage(loopError) and include its return value
in the logged message so the log uses the standardized error formatting.
In `@backend/src/entities/connection/use-cases/find-all-connections.use.case.ts`:
- Around line 109-117: The helper filterConnectionKeys is duplicated between
find-all-connections.use.case and find-one-connection.use.case; extract it into
a shared utility (e.g., filterConnectionKeys) that accepts (connection:
ConnectionEntity, allowedKeys: string[]) and returns FilteredConnection, export
it, then replace the local copies in both use-case files with an import of the
shared function and remove the duplicated definitions; ensure the utility uses
the same type imports (ConnectionEntity and FilteredConnection) and preserves
the existing behavior including the eslint disable comment.
In `@backend/src/entities/connection/use-cases/update-connection.use.case.ts`:
- Around line 65-76: The reduce callback is casting the accumulator on every
iteration; to optimize, create a correctly typed variable for the initial
accumulator before calling reduce (e.g., const initialAcc = {} as
UpdateConnectionDs['connection_parameters']) and pass that into
connection_parameters = Object.keys(connection_parameters).reduce(...,
initialAcc), then remove the per-iteration cast and use the accumulator
directly; keep existing checks involving params and keysToKeep and the same
property assignment logic inside the reducer.
In `@backend/src/entities/cron-jobs/cron-jobs.service.ts`:
- Around line 105-110: Replace the manual error normalization in the main cron
handler catch block with the shared helpers: call getErrorMessage(e) and
getErrorStack(e) to produce a consistent message and stack, then pass a combined
string to slackPostMessage(`Error in morning cron: ${message}\n${stack}`,
Constants.EXCEPTIONS_CHANNELS); still call Sentry.captureException(e) and keep
the console.error('Main cron handler error:', e) line; update the catch in
cron-jobs.service.ts to use getErrorMessage and getErrorStack instead of the
inline instanceof/Error conversion and stack slicing.
- Around line 96-104: Replace the manual error normalization and stack-slicing
with the existing helpers: call getErrorMessage(innerError) and
getErrorStack(innerError) to build the Slack message (e.g.,
`${getErrorMessage(innerError)}\n${getErrorStack(innerError)}` or slice the
stack lines via getErrorStack if needed), then pass the original innerError to
Sentry.captureException(innerError) and post to slackPostMessage(...,
Constants.EXCEPTIONS_CHANNELS); remove the ad-hoc new Error(...) creation and
manual stack handling to keep consistency with the error handling used at line
83.
In
`@backend/src/entities/table-filters/use-cases/update-table-filters.use.case.ts`:
- Around line 55-63: The duplicated loop that removes undefined properties from
entities (seen around updatedFilterEntity built by buildNewTableFiltersEntity in
update-table-filters.use.case.ts and the same pattern in
update-table-settings.use.case.ts) should be extracted to a small reusable
helper (e.g., removeUndefinedProperties<T extends object>(obj: T): T) exported
from a new helpers module; replace the inline loop in both use cases with a call
to removeUndefinedProperties(updatedFilterEntity) (and the equivalent variable
in the settings use case), keeping the same type signature and any necessary
eslint disable comments inside the helper so behavior and linting remain
identical.
In
`@backend/src/entities/table-settings/common-table-settings/repository/table-settings-custom-repository-extension.ts`:
- Line 57: Replace the direct cast (e as Error).message usage with the shared
getErrorMessage helper to safely extract the error text; import getErrorMessage
from backend/src/helpers/get-error-message.ts into
table-settings-custom-repository-extension.ts and update the catch/logging line
that currently calls console.info(`Table setting not found exception. => `, (e
as Error).message) to use getErrorMessage(e) so the log always prints a useful
message even when non-Error values are thrown.
In `@backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts`:
- Around line 122-130: Replace direct casts of (error as Error).message with the
centralized getErrorMessage(error) helper to ensure consistent, non-undefined
error text; update the Slack/debug message string interpolation and the
HttpException payload's originalMessage where Messages.CSV_EXPORT_FAILED is
thrown (inside the exportCsvFromTable use-case), and add/import getErrorMessage
at the top of the file if not already present so both logging and the
HttpException use the helper-derived message.
In
`@backend/src/entities/table/use-cases/find-tables-in-connection-v2.use.case.ts`:
- Around line 47-57: The catch block in the FindTablesInConnectionV2 use case
extracts the error message via (error as Error).message which breaks if a
non-Error is thrown; replace that extraction with a safe helper
(getErrorMessage(error)) and use the returned string for the comparisons against
Messages.MASTER_PASSWORD_MISSING and Messages.MASTER_PASSWORD_INCORRECT so all
thrown values are handled robustly and the existing HttpException branches (type
'no_master_key', HttpStatus.BAD_REQUEST, etc.) still run when appropriate.
In `@backend/src/shared/services/turnstile.service.ts`:
- Around line 46-47: Replace the ad-hoc axios-error casting and console.error
call with the standardized getErrorMessage helper (use getErrorMessage(error))
inside the Turnstile verification error handling so logs are consistent with
other services; if response.data truly contains extra diagnostics you need, keep
the existing err cast but add a short comment above it explaining why
axios-specific response.data is required for debugging and why we deviate from
getErrorMessage (reference the local err variable and the Turnstile verification
error log statement).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f5a94db-e71b-4778-9ca6-bd8ce35f21e6
📒 Files selected for processing (63)
.github/workflows/code-quality.ymlbackend/package.jsonbackend/src/entities/ai/ai.service.tsbackend/src/entities/ai/use-cases/request-info-from-table-with-ai-v7.use.case.tsbackend/src/entities/cedar-authorization/cedar-authorization.service.tsbackend/src/entities/cedar-authorization/cedar-schema.tsbackend/src/entities/connection/use-cases/find-all-connections.use.case.tsbackend/src/entities/connection/use-cases/find-one-connection.use.case.tsbackend/src/entities/connection/use-cases/get-connection-diagram.use.case.tsbackend/src/entities/connection/use-cases/preview-connection-diagram.use.case.tsbackend/src/entities/connection/use-cases/test-connection.use.case.tsbackend/src/entities/connection/use-cases/update-connection.use.case.tsbackend/src/entities/cron-jobs/cron-jobs.service.tsbackend/src/entities/demo-data/demo-data.service.tsbackend/src/entities/email/email/email.service.tsbackend/src/entities/shared-jobs/shared-jobs.service.tsbackend/src/entities/table-actions/table-action-rules-module/repository/action-rules-custom-repository.tsbackend/src/entities/table-actions/table-action-rules-module/use-cases/activate-actions-in-rule.use.case.tsbackend/src/entities/table-actions/table-actions-module/repository/table-actions-custom-repository.extension.tsbackend/src/entities/table-categories/use-cases/find-table-categories-with-tables.use.case.tsbackend/src/entities/table-filters/use-cases/update-table-filters.use.case.tsbackend/src/entities/table-logs/table-logs.controller.tsbackend/src/entities/table-logs/table-logs.service.tsbackend/src/entities/table-schema/use-cases/generate-schema-change.use-case.tsbackend/src/entities/table-schema/utils/cassandra-ddl.tsbackend/src/entities/table-schema/utils/elasticsearch-schema-op.tsbackend/src/entities/table-schema/utils/mongo-schema-op.tsbackend/src/entities/table-settings/common-table-settings/dto/create-table-settings.dto.tsbackend/src/entities/table-settings/common-table-settings/repository/table-settings-custom-repository-extension.tsbackend/src/entities/table-settings/common-table-settings/use-cases/update-table-settings.use.case.tsbackend/src/entities/table/use-cases/add-row-in-table.use.case.tsbackend/src/entities/table/use-cases/bulk-update-rows-in-table.use.case.tsbackend/src/entities/table/use-cases/delete-row-from-table.use.case.tsbackend/src/entities/table/use-cases/delete-rows-from-table.use.case.tsbackend/src/entities/table/use-cases/export-csv-from-table.use.case.tsbackend/src/entities/table/use-cases/find-tables-in-connection-v2.use.case.tsbackend/src/entities/table/use-cases/find-tables-in-connection.use.case.tsbackend/src/entities/table/use-cases/get-row-by-primary-key.use.case.tsbackend/src/entities/table/use-cases/get-table-rows.use.case.tsbackend/src/entities/table/use-cases/get-table-structure-without-cache.use.case.tsbackend/src/entities/table/use-cases/get-table-structure.use.case.tsbackend/src/entities/table/use-cases/import-csv-in-table-user.case.tsbackend/src/entities/table/use-cases/update-row-in-table.use.case.tsbackend/src/entities/table/utils/find-objects-with-properties.tsbackend/src/entities/user-actions/use-cases/check-users-actions-and-mailing-users.use.case.tsbackend/src/entities/user/use-cases/log-out.use.case.tsbackend/src/entities/visualizations/panel-position/use-cases/generate-panel-position-with-ai.use.case.tsbackend/src/entities/visualizations/panel-position/use-cases/generate-table-dashboard-with-ai.use.case.tsbackend/src/entities/visualizations/panel/panel.entity.tsbackend/src/entities/widget/table-widget.entity.tsbackend/src/helpers/constants/constants.tsbackend/src/helpers/get-error-message.tsbackend/src/helpers/validators/is-action-url-host-allowed.tsbackend/src/microservices/saas-microservice/use-cases/create-connection-for-hosted-db.use.case.tsbackend/src/shared/config/app-config.tsbackend/src/shared/services/turnstile.service.tsbackend/src/types/basic-auth.d.tsbackend/test/ava-tests/connection-diagram-preview.test.tsbackend/test/ava-tests/saas-tests/custom-domains-e2e.test.tsbackend/test/ava-tests/saas-tests/table-widgets-e2e.test.tsbackend/test/utils/create-test-table.tsbackend/tsconfig.jsonbackend/tsconfig.src.json
Comment on lines
44
to
47
| } catch (error) { | ||
| console.error(`Error during demo data creation for user with ID ${userId}:`, error); | ||
| await slackPostMessage(`Error during demo data creation for user with ID ${userId}: ${error.message}`); | ||
| await slackPostMessage(`Error during demo data creation for user with ID ${userId}: ${(error as Error).message}`); | ||
| } |
There was a problem hiding this comment.
Do not swallow demo-data creation failures.
createDemoDataForUser currently resolves with undefined on error, which can silently mask provisioning failures for callers expecting Array<ConnectionEntity>. Re-throw after reporting.
💡 Suggested fix
} catch (error) {
console.error(`Error during demo data creation for user with ID ${userId}:`, error);
- await slackPostMessage(`Error during demo data creation for user with ID ${userId}: ${(error as Error).message}`);
+ const message = error instanceof Error ? error.message : String(error);
+ await slackPostMessage(`Error during demo data creation for user with ID ${userId}: ${message}`);
+ throw error;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/demo-data/demo-data.service.ts` around lines 44 - 47,
The catch block in createDemoDataForUser currently logs and posts to Slack but
swallows the error, causing the function to resolve with undefined instead of
propagating failures; update the catch in createDemoDataForUser to re-throw the
caught error (after console.error and await slackPostMessage) so callers receive
the exception and the function's contract (Array<ConnectionEntity>) isn’t
silently broken.
| @Get('/logs/:connectionId') | ||
| async findAll( | ||
| @Query() query, | ||
| @Query() query: Record<string, string>, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In NestJS, what runtime shapes can @query() have for repeated query parameters (e.g., ?a=1&a=2), and what TypeScript type is recommended for controller method signatures?
💡 Result:
For repeated query parameters (e.g.?a=1&a=2), the runtime shape you’ll typically see with NestJS’s @Query('key') is: 1) If you declare the parameter as an array in your controller signature (e.g. @Query('a') a: string[]), the value is treated as an array of the provided occurrences: ['1','2']. 2) If the client sends only a single occurrence (?a=1), the same parameter may arrive as a single string instead of a 1-element array unless you normalize/transform it. TypeScript recommendation for controller method signatures: Use an explicit DTO (class) or an inline object type for @Query to describe expected query parameters, and for repeated keys use the array type, e.g. { a?: string[] }. NestJS encourages extracting query parameters via @Query either individually or as a whole object and documenting/typing arrays with array types in @Query/@apiquery contexts [1][2]. Example signatures: - For a whole query object: @Get findAll(@Query query: { a?: string[] }) {} - For a single repeated key: @Get findAll(@Query('a') a: string[]) {} Notes: - NestJS itself relies on the underlying HTTP adapter’s query parser for how it builds req.query; Express v5 defaults to a “simple” parser (no nesting/arrays), and the Nest migration notes explain that to get the previous behavior for array-like syntaxes you must set the Express query parser to 'extended' [3].
Citations:
- 1: https://docs.nestjs.com/controllers
- 2: https://docs.nestjs.com/openapi/types-and-parameters
- 3: https://github.com/nestjs/docs.nestjs.com/blob/master/content/migration.md
Update @Query() typing to handle repeated query keys
Repeated query parameters (e.g. ?a=1&a=2) can yield string[] values at runtime, so @Query() query: Record<string, string> can mismatch the actual shape. Prefer Record<string, string | string[]> (or a dedicated DTO that models array-valued params) for table-logs.controller.ts. Apply to any other @Query() params typed the same way in this controller.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/table-logs/table-logs.controller.ts` at line 71, The
`@Query`() parameter typing in table-logs.controller.ts (the parameter named
query) wrongly assumes all query values are strings; change its type from
Record<string, string> to Record<string, string | string[]> (or replace with a
DTO that models array-valued params) so repeated query keys like ?a=1&a=2 are
handled; update any other `@Query`() parameters in this controller that use
Record<string, string> to the same Record<string, string | string[]> (or
corresponding DTO) to keep typings consistent.
Comment on lines
+197
to
+200
| message: (e as Error).message.includes('duplicate key value') | ||
| ? Messages.CANT_INSERT_DUPLICATE_KEY | ||
| : `${Messages.FAILED_ADD_ROW_IN_TABLE} | ||
| ${Messages.ERROR_MESSAGE} ${e.message} ${Messages.TRY_AGAIN_LATER}`, | ||
| ${Messages.ERROR_MESSAGE} ${(e as Error).message} ${Messages.TRY_AGAIN_LATER}`, |
There was a problem hiding this comment.
Unsafe error message access could cause secondary failure in error handling.
If a non-Error value is thrown, (e as Error).message will be undefined, causing undefined.includes('duplicate key value') to throw a TypeError. This means the error handler itself could crash, preventing proper error reporting to the user.
While most database operations throw Error objects, using the getErrorMessage helper introduced in this PR would eliminate this runtime hazard and provide more robust error handling.
🛡️ Safer error message extraction
+import { getErrorMessage } from '../../../helpers/get-error-message.js';
} catch (e) {
operationResult = OperationResultStatusEnum.unsuccessfully;
+ const errorMessage = getErrorMessage(e);
throw new HttpException(
{
- message: (e as Error).message.includes('duplicate key value')
+ message: errorMessage.includes('duplicate key value')
? Messages.CANT_INSERT_DUPLICATE_KEY
: `${Messages.FAILED_ADD_ROW_IN_TABLE}
- ${Messages.ERROR_MESSAGE} ${(e as Error).message} ${Messages.TRY_AGAIN_LATER}`,
+ ${Messages.ERROR_MESSAGE} ${errorMessage} ${Messages.TRY_AGAIN_LATER}`,
},
HttpStatus.BAD_REQUEST,
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/table/use-cases/add-row-in-table.use.case.ts` around
lines 197 - 200, The error handler in add-row-in-table.use.case.ts currently
directly accesses (e as Error).message and calls .includes which can throw if a
non-Error is thrown; replace that direct access with the shared getErrorMessage
helper to safely extract the message, then perform the duplicate-key check
against the returned string (e.g., const msg = getErrorMessage(e); if
(msg.includes('duplicate key value')) ...) and use msg when composing the
fallback Messages.FAILED_ADD_ROW_IN_TABLE error text so the handler cannot crash
when a non-Error is thrown.
| } | ||
| } catch (error) { | ||
| result = encodeError({ error: error.message }); | ||
| result = encodeError({ error: (error as Error).message }); |
There was a problem hiding this comment.
Use the getErrorMessage helper for safer error message extraction.
Casting caught errors directly to Error assumes the thrown value is always an Error object. JavaScript allows throwing any value (strings, numbers, objects, null), which could cause runtime failures if .message doesn't exist.
The PR introduces a getErrorMessage helper in backend/src/helpers/get-error-message.ts specifically for safe error message extraction from unknown-typed catch values. Use it consistently across the codebase instead of manual type assertions.
🛡️ Recommended approach using the safe helper
First, add the import at the top of the file:
+import { getErrorMessage } from '../../../../helpers/get-error-message.js';Then update the three catch blocks:
} catch (error) {
- result = encodeError({ error: (error as Error).message });
+ result = encodeError({ error: getErrorMessage(error) });
} } catch (error) {
- this.logger.error('Error parsing AI response:', (error as Error).message);
+ this.logger.error('Error parsing AI response:', getErrorMessage(error));
throw new BadRequestException( } catch (error) {
- return { success: false, error: (error as Error).message };
+ return { success: false, error: getErrorMessage(error) };
}Also applies to: 309-309, 385-385
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/entities/visualizations/panel-position/use-cases/generate-panel-position-with-ai.use.case.ts`
at line 288, Replace unsafe casts of caught errors to Error with the shared
helper: import getErrorMessage from 'backend/src/helpers/get-error-message' at
the top of generate-panel-position-with-ai.use.case.ts, then change any
occurrences that set result = encodeError({ error: (error as Error).message })
(and the similar lines around the catch blocks near the current diff positions)
to use encodeError({ error: getErrorMessage(error) }) or simply
encodeError(getErrorMessage(error)) consistent with existing encoding patterns;
update all three catch sites (the ones flagged at the current diff and the ones
around lines 309 and 385) so they call getErrorMessage(error) instead of casting
to Error.
| }); | ||
| } catch (error) { | ||
| this.logger.warn(`Panel "${panel.name}" skipped: ${error.message}`); | ||
| this.logger.warn(`Panel "${panel.name}" skipped: ${(error as Error).message}`); |
There was a problem hiding this comment.
Use the getErrorMessage helper for safer error message extraction.
Manual type assertions (error as Error).message assume the caught value is always an Error object, but JavaScript permits throwing any value. Use the getErrorMessage helper introduced in this PR for consistent, safe error message extraction.
🛡️ Recommended approach using the safe helper
Add the import:
+import { getErrorMessage } from '../../../../helpers/get-error-message.js';Update all four catch blocks:
} catch (error) {
- this.logger.warn(`Panel "${panel.name}" skipped: ${(error as Error).message}`);
+ this.logger.warn(`Panel "${panel.name}" skipped: ${getErrorMessage(error)}`);
} } catch (error) {
- result = encodeError({ error: (error as Error).message });
+ result = encodeError({ error: getErrorMessage(error) });
} } catch (error) {
- this.logger.error('Error parsing dashboard AI response:', (error as Error).message);
+ this.logger.error('Error parsing dashboard AI response:', getErrorMessage(error));
throw new BadRequestException('Failed to generate dashboard from AI. Please try again.'); } catch (error) {
- return { success: false, error: (error as Error).message };
+ return { success: false, error: getErrorMessage(error) };
}Also applies to: 356-356, 397-397, 457-457
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/entities/visualizations/panel-position/use-cases/generate-table-dashboard-with-ai.use.case.ts`
at line 164, Replace unsafe casts of caught exceptions like (error as
Error).message with the safe getErrorMessage helper: import getErrorMessage from
its module at the top of generate-table-dashboard-with-ai.use.case.ts, then
update each catch block that currently does this.logger.warn(`Panel
"${panel.name}" skipped: ${(error as Error).message}`) (and the three other
similar logger.warn calls in the same file) to call getErrorMessage(error) and
include its return value in the log message instead; ensure all four occurrences
use getErrorMessage for consistent, safe error extraction.
| } | ||
| } catch (e) { | ||
| console.error('-> Error widget options stringify ' + e.message); | ||
| console.error('-> Error widget options stringify ' + (e as Error).message); |
There was a problem hiding this comment.
Use the getErrorMessage helper for safer error message extraction.
The entity lifecycle hooks cast caught errors to Error before accessing .message. This assumption is unsafe since any value can be thrown in JavaScript. Use the getErrorMessage helper for consistent, safe error message extraction.
🛡️ Recommended approach using the safe helper
Add the import at the top:
import sjson from 'secure-json-parse';
+import { getErrorMessage } from '../../../helpers/get-error-message.js';
import {Update all three lifecycle hooks:
} catch (e) {
- console.error('-> Error widget options stringify ' + (e as Error).message);
+ console.error('-> Error widget options stringify ' + getErrorMessage(e));
} } catch (e) {
- console.error('-> Error widget options stringify ' + (e as Error).message);
+ console.error('-> Error widget options stringify ' + getErrorMessage(e));
} } catch (e) {
- console.error('-> Error widget options parse ' + (e as Error).message);
+ console.error('-> Error widget options parse ' + getErrorMessage(e));
}Also applies to: 73-73, 90-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/visualizations/panel/panel.entity.ts` at line 58,
Replace unsafe casts to Error in the entity lifecycle hooks by importing and
using the getErrorMessage helper; add an import for getErrorMessage at the top
of panel.entity.ts and in the three lifecycle hook methods (the ones that
currently call console.error('-> Error widget options stringify ' + (e as
Error).message) and the two similar lines) change those message constructions to
use getErrorMessage(e) so the log calls become console.error('-> Error widget
options stringify ' + getErrorMessage(e)) (apply the same replacement for the
other two hooks).
| } | ||
| } catch (e) { | ||
| console.error('-> Error widget options stringify ' + e.message); | ||
| console.error('-> Error widget options stringify ' + (e as Error).message); |
There was a problem hiding this comment.
Use the getErrorMessage helper for safer error message extraction.
The lifecycle hooks manually cast caught errors to Error, which is unsafe since JavaScript allows throwing any value. Use the getErrorMessage helper for consistent, safe error message extraction across the codebase.
🛡️ Recommended approach using the safe helper
Add the import:
import sjson from 'secure-json-parse';
+import { getErrorMessage } from '../../helpers/get-error-message.js';
import {Update the three hooks:
} catch (e) {
- console.error('-> Error widget options stringify ' + (e as Error).message);
+ console.error('-> Error widget options stringify ' + getErrorMessage(e));
} } catch (e) {
- console.error('-> Error widget options stringify ' + (e as Error).message);
+ console.error('-> Error widget options stringify ' + getErrorMessage(e));
} } catch (e) {
- console.error('-> Error widget options parse ' + (e as Error).message);
+ console.error('-> Error widget options parse ' + getErrorMessage(e));
}Also applies to: 65-65, 79-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/widget/table-widget.entity.ts` at line 54, Replace the
unsafe manual error casts in the TableWidget entity lifecycle hooks with the
shared getErrorMessage helper: add an import for getErrorMessage and update the
three places where errors are logged (the console.error call at the location
shown and the two other lifecycle-hook error logs referenced around the same
file) to call getErrorMessage(e) instead of casting (e as Error). Ensure each
log message uses the helper to extract the message and keep the existing
descriptive text.
| }); | ||
| } catch (e) { | ||
| console.error('Invalid URL format for action URL validation:', e.message); | ||
| console.error('Invalid URL format for action URL validation:', (e as Error).message); |
There was a problem hiding this comment.
Use the getErrorMessage helper for safer error message extraction.
Directly casting to Error assumes the thrown value is always an Error object. Use the getErrorMessage helper for safe message extraction from unknown-typed catch values.
🛡️ Recommended fix
Add the import:
import dns from 'dns';
import ipRangeCheck from 'ip-range-check';
+import { getErrorMessage } from '../get-error-message.js';
import { isSaaS } from '../app/is-saas.js';Update the catch block:
} catch (e) {
- console.error('Invalid URL format for action URL validation:', (e as Error).message);
+ console.error('Invalid URL format for action URL validation:', getErrorMessage(e));
return false;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/helpers/validators/is-action-url-host-allowed.ts` at line 40,
Replace the direct cast in the catch block with the safe helper: add an import
for getErrorMessage at the top of the module and change the console.error call
inside the catch in is-action-url-host-allowed (the catch that currently logs (e
as Error).message) to use getErrorMessage(e) instead so unknown thrown values
are handled safely.
| console.log('PGLite directory is writable'); | ||
| } catch (writeError) { | ||
| console.warn('PGLite directory exists but may not be writable:', writeError.message); | ||
| console.warn('PGLite directory exists but may not be writable:', (writeError as Error).message); |
There was a problem hiding this comment.
Use the getErrorMessage helper for safer error message extraction.
The manual cast (writeError as Error).message is unsafe. Use the getErrorMessage helper to handle cases where the caught value might not be an Error object.
🛡️ Recommended fix
Add the import at the top:
import { uuid_ossp } from '`@electric-sql/pglite/contrib/uuid_ossp`';
import dotenv from 'dotenv';
import fs from 'fs';
+import { getErrorMessage } from '../../helpers/get-error-message.js';
import path, { join } from 'path';Update the warning:
} catch (writeError) {
- console.warn('PGLite directory exists but may not be writable:', (writeError as Error).message);
+ console.warn('PGLite directory exists but may not be writable:', getErrorMessage(writeError));
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/shared/config/app-config.ts` at line 279, Replace the unsafe cast
(writeError as Error).message with the safer helper getErrorMessage: import
getErrorMessage at the top, then call getErrorMessage(writeError) inside the
console.warn so the warning uses getErrorMessage(writeError) instead of
(writeError as Error).message; this keeps the existing message text and context
but extracts the error string robustly for the writeError variable in the same
block.
| t.is(Object.keys(registerDomainResponseRO).length, 5); | ||
| } catch (error) { | ||
| t.fail(error.message); | ||
| t.fail((error as Error).message); |
There was a problem hiding this comment.
Use the getErrorMessage helper in test error handlers.
All eight test catch blocks cast errors to Error before accessing .message. Use the getErrorMessage helper for consistent, safe error message extraction throughout the codebase, including tests.
🛡️ Recommended fix for all test cases
Add the import at the top:
import { ApplicationModule } from '../../../src/app.module.js';
// import nock from 'nock';
+import { getErrorMessage } from '../../../src/helpers/get-error-message.js';
import { WinstonLogger } from '../../../src/entities/logging/winston-logger.js';Update all eight catch blocks:
} catch (error) {
- t.fail((error as Error).message);
+ t.fail(getErrorMessage(error));
}Also applies to: 169-169, 246-246, 299-299, 365-365, 421-421, 478-478, 549-549
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/test/ava-tests/saas-tests/custom-domains-e2e.test.ts` at line 131,
Replace direct casts to Error in the test catch blocks with the shared helper:
import getErrorMessage at the top of
backend/test/ava-tests/saas-tests/custom-domains-e2e.test.ts and change each
t.fail((error as Error).message) to t.fail(getErrorMessage(error)); update all
eight catch sites (current occurrences at the indicated lines) so tests use
getErrorMessage(error) for safe, consistent error extraction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Refactor
Chores